Add parameter to ExtModule for requirements#5174
Conversation
trmckay
force-pushed
the
extmodule-require
branch
from
0560b6a to
32c502a
Compare
trmckay
force-pushed
the
extmodule-require
branch
2 times, most recently
from
2359f0a to
85a7ae7
Compare
src/main/scala/chisel3/stage/phases/AddDedupGroupAnnotations.scala
Outdated
Show resolved
Hide resolved
jackkoenig
left a comment
jackkoenig
left a comment
There was a problem hiding this comment.
Generally LGTM, I think we should wait until a firtool release with requires support (and bump) before merging. I want to do a Chisel release soon and might do so before firtool supports this.
trmckay
force-pushed
the
extmodule-require
branch
from
2db572d to
afdc23f
Compare
|
@trmckay firtool has been bumped, maybe you can retarget this to |
| importedDefinition.proto.layers | ||
| importedDefinition.proto.layers, | ||
| // Imported Definitions necessarily don't have external requirements | ||
| Seq.empty |
There was a problem hiding this comment.
They don't necessarily but they might right?
There was a problem hiding this comment.
Correct, they might contain modules with external requirements. But the module body of the imported Definition cannot since it would be generated by Chisel rather than being an extmodule of arbitrary verilog.
You would want to look at the instance graph in the circuit of the imported definition to find external requirements of those modules (none of which would correspond directly to the imported definition itself).
When you did the same for the instance graph in the circuit to which you imported the definition, you would find that it depends on an extmodule for that definition.
Combining the two results, you have everything you need to compile the second circuit.
2 participants
Adds an API for the
requireskeyword supported by CIRCT after llvm/circt#9496.Contributor Checklist
docs/src?Type of Improvement
Desired Merge Strategy
Release Notes
requirementsparameter toExtModulefor tracking external build requirements.Reviewer Checklist (only modified by reviewer)
3.6.x,5.x, or6.xdepending on impact, API modification or big change:7.0)?Enable auto-merge (squash)and clean up the commit message.Create a merge commit.